Skip to content

Conversation

nojaf
Copy link
Member

@nojaf nojaf commented Sep 22, 2025

Fixes #7718

Copy link

pkg-pr-new bot commented Sep 22, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7916

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7916

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7916

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7916

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7916

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7916

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7916

commit: aefd238

@nojaf
Copy link
Member Author

nojaf commented Sep 22, 2025

@cknitt you can already try this out.
Note that, as with the other commands, we now take two dashes --warn-error

@nojaf nojaf marked this pull request as ready for review September 22, 2025 11:16
@fhammerschmidt
Copy link
Member

Should we have -warn-error with one dash as well but immediately mark it as deprecated, like we have with -w?

@nojaf
Copy link
Member Author

nojaf commented Sep 22, 2025

Should we have -warn-error with one dash as well but immediately mark it as deprecated, like we have with -w?

I'm against this idea. These hacks aren't worth it to me. It's a major version; adding a single dash is not the end of the world. Is there any evidence this is a widely used feature? For free software, I think it's a small price to pay.

@cknitt
Copy link
Member

cknitt commented Sep 23, 2025

@nojaf Thanks! Just tested, and works great. 🎉

@nojaf nojaf added the build label Sep 26, 2025
@cknitt
Copy link
Member

cknitt commented Sep 26, 2025

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

@nojaf nojaf requested a review from Copilot September 26, 2025 12:48
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for a --warn-error command-line flag to the build and watch commands in Rewatch, allowing users to override warning configurations from rescript.json. The flag follows the same precedence behavior as the legacy bsb build system.

  • Adds --warn-error CLI flag to both build and watch commands
  • Implements warning override logic that takes precedence over rescript.json configuration
  • Threads the warning parameter through the entire build system from CLI to compiler argument generation
  • Includes comprehensive test coverage and integration tests

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rewatch/src/cli.rs Adds --warn-error field to BuildArgs and WatchArgs structs
rewatch/src/main.rs Passes the warn_error parameter to build and watch functions
rewatch/src/watcher.rs Updates watcher to accept and thread the warn_error parameter
rewatch/src/config.rs Implements warning override logic and adds comprehensive unit tests
rewatch/src/build/ Updates build system components to handle the new BuildCommandState wrapper
rewatch/tests/compiler-args.sh Adds integration test to verify warning flags appear in both parser and compiler args
rewatch/testrepo/packages/namespace-casing/rescript.json Adds test configuration with warning settings

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@nojaf nojaf enabled auto-merge (squash) September 26, 2025 13:16
@nojaf nojaf merged commit c0aaede into rescript-lang:master Sep 26, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewatch: no way to override warnings as errors on command line

4 participants